Hook iOS FlutterSpellCheckPlugin to Framework #35742
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Tests added/fixed, removing need test label |
| NSDictionary* suggestionsJSON1 = capturedResult.firstObject; | ||
| XCTAssertEqualObjects(suggestionsJSON1[@"startIndex"], @0); | ||
| XCTAssertEqualObjects(suggestionsJSON1[@"endIndex"], @4); | ||
| XCTAssertEqualObjects(suggestionsJSON1[@"endIndex"], @5); |
There was a problem hiding this comment.
Is this right?
This isss misspelled
xxxxx^^^^xx
01234567890
Start index is 5, length is 4, end index is 8, not 9.
There was a problem hiding this comment.
There was a problem hiding this comment.
https://api.flutter.dev/flutter/dart-ui/TextRange-class.html
The next index after the characters in this range.
Well that's confusing, as you experienced from the off by one error... Can you add a comment about that in toDictionary? In Objective-C this is usually expressed with a tuple range of start index + length, that the endIndex is after the range is not intuitive.
There was a problem hiding this comment.
Test Case '-[FlutterSpellCheckPluginTest testFindAllSpellCheckSuggestionsForText]' started.
/../../flutter/shell/platform/darwin/ios/framework/Source/FlutterSpellCheckPluginTest.mm:164: error: -[FlutterSpellCheckPluginTest testFindAllSpellCheckSuggestionsForText] : ((suggestionsJSON1[@"endIndex"]) equal to (@4)) failed: ("5") is not equal to ("4")
/../../flutter/shell/platform/darwin/ios/framework/Source/FlutterSpellCheckPluginTest.mm:168: error: -[FlutterSpellCheckPluginTest testFindAllSpellCheckSuggestionsForText] : ((suggestionsJSON2[@"endIndex"]) equal to (@9)) failed: ("10") is not equal to ("9")
Test Case '-[FlutterSpellCheckPluginTest testFindAllSpellCheckSuggestionsForText]' failed (0.061 seconds).
|
Unfortunately I think you're hitting something relating to #35843, can you rebase onto top of tree? |
|
From PR triage: It looks like this PR needs to be rebased as @jmagman says. |
|
Now you're hitting flutter/flutter#111193 😞 you'll have to wait until that's fixed and then rebase or merge again |
camsim99
added
the
autosubmit
4 participants
Fixes the following to make the iOS
FlutterSpellCheckPlugincompatible with the framework:nativeSpellCheckServiceDefinedto true since iOS supports spell check.Also removes unnecessary line from Android
SpellCheckPluginI encountered.Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.